-
Notifications
You must be signed in to change notification settings - Fork 0
添加依赖注入单元测试,规范化代码。 #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
审阅者指南此 PR 将 hook 标识符重构为枚举类型,简化了 hook 管理和 UUID 处理,合并了默认货币常量,提升了包版本并添加了新的适配器依赖,同时增加了依赖注入单元测试套件。 重构后的 HooksType 和 HooksManager 的类图classDiagram
class HooksType {
<<Enum>>
PRE: str = "vault_pre_transaction"
POST: str = "vault_post_transaction"
+pre() str
+post() str
+methods() list[str]
}
class HooksManager {
-__hooks: dict[str, list[Callable[..., Awaitable[None]]]]
-_instance
+__new__()
+register(hook_name: str, hook_func: Callable[..., Awaitable[None]])
+run_hooks(hook_name: str, context: Any)
}
HooksManager --> HooksType : uses
更新后的 UUID 和默认货币处理的类图classDiagram
class uuid_lib {
+NAMESPACE_VALUE: UUID
+DEFAULT_NAME: str
+DEFAULT_CURRENCY_UUID: UUID
+to_uuid(s: str) str
+get_uni_id(id: str, currency_id: str) str
}
class CurrencyRepository {
+DEFAULT_CURRENCY_UUID: UUID
+DEFAULT_NAME: str
+NAMESPACE_VALUE: UUID
}
class UserAccount {
+__init__(**kwargs: Any)
}
uuid_lib <.. CurrencyRepository : provides constants
uuid_lib <.. UserAccount : provides get_uni_id, NAMESPACE_VALUE
清理后的 Method 枚举类图classDiagram
class Method {
<<Enum>>
DEPOSIT: str
WITHDRAW: str
TRANSFER_IN: str
TRANSFER_OUT: str
}
文件级变更
提示和命令与 Sourcery 交互
自定义您的体验访问您的 仪表板 以:
获取帮助Original review guide in EnglishReviewer's GuideThis PR refactors hook identifiers into an Enum, streamlines hook management and UUID handling, consolidates default currency constants, bumps the package version with a new adapter dependency, and adds a dependency‐injection unit test suite. Class diagram for refactored HooksType and HooksManagerclassDiagram
class HooksType {
<<Enum>>
PRE: str = "vault_pre_transaction"
POST: str = "vault_post_transaction"
+pre() str
+post() str
+methods() list[str]
}
class HooksManager {
-__hooks: dict[str, list[Callable[..., Awaitable[None]]]]
-_instance
+__new__()
+register(hook_name: str, hook_func: Callable[..., Awaitable[None]])
+run_hooks(hook_name: str, context: Any)
}
HooksManager --> HooksType : uses
Class diagram for updated UUID and default currency handlingclassDiagram
class uuid_lib {
+NAMESPACE_VALUE: UUID
+DEFAULT_NAME: str
+DEFAULT_CURRENCY_UUID: UUID
+to_uuid(s: str) str
+get_uni_id(id: str, currency_id: str) str
}
class CurrencyRepository {
+DEFAULT_CURRENCY_UUID: UUID
+DEFAULT_NAME: str
+NAMESPACE_VALUE: UUID
}
class UserAccount {
+__init__(**kwargs: Any)
}
uuid_lib <.. CurrencyRepository : provides constants
uuid_lib <.. UserAccount : provides get_uni_id, NAMESPACE_VALUE
Class diagram for Method Enum after cleanupclassDiagram
class Method {
<<Enum>>
DEPOSIT: str
WITHDRAW: str
TRANSFER_IN: str
TRANSFER_OUT: str
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
嗨 @JohnRichard4096 - 我已审阅您的更改 - 以下是一些反馈:
- 在 HooksManager.register 中,
if hook_name not in HooksType:无法正确验证字符串钩子名称——请考虑检查HooksType._value2member_map_或使用HooksType(hook_name)进行验证。 - HooksType 枚举仍然提供
pre和post方法,但大多数代码现在使用.value;请考虑删除这些方法或重构为单个一致的 API 来检索钩子标识符。 - 在 HooksType 中添加一个类方法,返回其所有字符串值(例如
.values()),然后将其用于验证和methods()中,以避免分散的.value访问。
AI 代理提示
请处理此代码审查中的评论:
## 总体评论
- 在 HooksManager.register 中,`if hook_name not in HooksType:` 无法正确验证字符串钩子名称——请考虑检查 `HooksType._value2member_map_` 或使用 `HooksType(hook_name)` 进行验证。
- HooksType 枚举仍然提供 `pre` 和 `post` 方法,但大多数代码现在使用 `.value`;请考虑删除这些方法或重构为单个一致的 API 来检索钩子标识符。
- 在 HooksType 中添加一个类方法,返回其所有字符串值(例如 `.values()`),然后将其用于验证和 `methods()` 中,以避免分散的 `.value` 访问。
## 单独评论
### 评论 1
<location> `nonebot_plugin_value/hook/hooks_type.py:4` </location>
<code_context>
+from enum import Enum
+
+
+class HooksType(Enum, str):
+ PRE = "vault_pre_transaction"
+ POST = "vault_post_transaction"
</code_context>
<issue_to_address>
枚举继承顺序应为 (str, Enum) 以获得正确行为。
当前的 (Enum, str) 顺序可能导致类型检查和序列化问题。请切换到 (str, Enum) 以确保正确的字符串行为。
</issue_to_address>
### 评论 2
<location> `nonebot_plugin_value/hook/hooks_type.py` </location>
<code_context>
+ @classmethod
+ def pre(cls) -> str:
+ return cls.PRE
</code_context>
<issue_to_address>
返回枚举成员而不是其值可能会导致问题。
目前,这些方法返回枚举成员而不是其字符串值,这可能会破坏期望字符串的代码。请返回 cls.PRE.value 以保持一致性。
</issue_to_address>
### 评论 3
<location> `nonebot_plugin_value/hook/hooks_type.py` </location>
<code_context>
+ @classmethod
+ def post(cls) -> str:
+ return cls.POST
</code_context>
<issue_to_address>
post() 应返回枚举值,而不是成员。
返回 cls.POST.value 以确保方法返回预期的字符串,与 pre() 的行为匹配。
</issue_to_address>帮助我更有用!请点击 👍 或 👎 对每个评论进行反馈,我将使用反馈来改进您的评论。
Original comment in English
Hey @JohnRichard4096 - I've reviewed your changes - here's some feedback:
- In HooksManager.register,
if hook_name not in HooksType:won’t correctly validate string hook names—consider checking against HooksType.value2member_map or usingHooksType(hook_name)for validation. - The HooksType enum still provides
preandpostmethods but most code now uses.value; consider removing those methods or refactoring to a single consistent API for retrieving hook identifiers. - Add a classmethod to HooksType that returns all its string values (e.g.
.values()), then use it for validation and inmethods()to avoid scattered.valueaccess.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In HooksManager.register, `if hook_name not in HooksType:` won’t correctly validate string hook names—consider checking against HooksType._value2member_map_ or using `HooksType(hook_name)` for validation.
- The HooksType enum still provides `pre` and `post` methods but most code now uses `.value`; consider removing those methods or refactoring to a single consistent API for retrieving hook identifiers.
- Add a classmethod to HooksType that returns all its string values (e.g. `.values()`), then use it for validation and in `methods()` to avoid scattered `.value` access.
## Individual Comments
### Comment 1
<location> `nonebot_plugin_value/hook/hooks_type.py:4` </location>
<code_context>
+from enum import Enum
+
+
+class HooksType(Enum, str):
+ PRE = "vault_pre_transaction"
+ POST = "vault_post_transaction"
</code_context>
<issue_to_address>
Enum inheritance order should be (str, Enum) for correct behavior.
The current (Enum, str) order can cause issues with type checks and serialization. Please switch to (str, Enum) to ensure correct string behavior.
</issue_to_address>
### Comment 2
<location> `nonebot_plugin_value/hook/hooks_type.py` </location>
<code_context>
+ @classmethod
+ def pre(cls) -> str:
+ return cls.PRE
</code_context>
<issue_to_address>
Returning enum members instead of their values may cause issues.
Currently, these methods return the enum member instead of its string value, which may break code expecting a string. Please return cls.PRE.value for consistency.
</issue_to_address>
### Comment 3
<location> `nonebot_plugin_value/hook/hooks_type.py` </location>
<code_context>
+ @classmethod
+ def post(cls) -> str:
+ return cls.POST
</code_context>
<issue_to_address>
post() should return the enum value, not the member.
Return cls.POST.value to ensure the method returns the expected string, matching the behavior of pre().
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sourcery 总结
重构 hook 类型和管理器,集中化货币 UUID 常量,简化 ID 生成,更新依赖并提升版本,并为账户操作添加依赖注入单元测试。
功能增强:
构建:
测试:
Original summary in English
Summary by Sourcery
Refactor hook types and manager, centralize currency UUID constants, streamline ID generation, bump version with updated dependencies, and add dependency-injection unit tests for account operations
Enhancements:
Build:
Tests: